-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#2532][#2512] fix(IT): fix the web e2e test errors when running on the github actions #2534
Conversation
What is the exact issue here, can we just add more descriptions about the problem and how do we fix it? |
37064ac
to
cefcdc2
Compare
...est/java/com/datastrato/gravitino/integration/test/web/ui/utils/ChromeWebDriverProvider.java
Outdated
Show resolved
Hide resolved
…loading for each small module (#2574) ### What changes were proposed in this pull request? 1. Displaying a fallback while content is loading for the main content Displaying a fallback while content is loading for metalakes list table body module Displaying a fallback while content is loading for catalog tree list modul ![UILoading](https://github.com/datastrato/gravitino/assets/9210625/0911c818-ee1b-400d-9b58-3af764a59363) 2. Define metadata icon src path based on the operating environment 3. Fix some issue of inconsistent behaviour - Smooth display of loaded data - Preserve search parameters when strongly refreshing a page ![fixbugs](https://github.com/datastrato/gravitino/assets/9210625/dd94fce7-4ee0-4c76-a40e-94e4a87f5ecc) ### Why are the changes needed? An instant loading state is fallback UI that is shown immediately upon navigation. You can pre-render loading indicators such as skeletons and spinners. This helps users understand the app is responding and provides a better user experience. Fix issue because of the need for consistent behaviour after refactoring Fix: #2570, #2559 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Will run e2e test after PR #2534 merged --------- Co-authored-by: CHEYNE <[email protected]>
d3cf71b
to
15d2509
Compare
LGTM, Thanks @ch3yne |
Actions action = new Actions(driver); | ||
action.moveToElement(element).click().build().perform(); | ||
Thread.sleep(ACTION_SLEEP_MILLIS); | ||
LOG.error(e.getMessage(), e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not throw exception? only print error log.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no need to print error messages here, I removed that line and added a comment.
@BeforeEach | ||
public void beforeEachTest() { | ||
try { | ||
Thread.sleep(ACTION_SLEEP_MILLIS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think better change variable name ACTION_SLEEP_MILLIS
to EACH_TEST_SLEEP_MILLIS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -89,8 +84,14 @@ public void setMetalakeCommentField(String commentField) { | |||
} | |||
|
|||
public void setQueryInput(String queryInput) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think better change function and variable name QueryInput
to QueryParams
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
// } | ||
public boolean verifyLinkToCatalogsPage(String name) { | ||
try { | ||
String xpath = "//a[@data-refer='metalake-name-link']"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define a link element variable in class member?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
WebElement noMetalakeRows = driver.findElement(By.xpath(xpath)); | ||
try { | ||
String xpath = | ||
"//div[@data-refer='metalake-table-grid']//div[contains(@class, 'MuiDataGrid-overlay')]"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define a link element variable in class member?
public boolean checkIsErrorName() throws InterruptedException { | ||
try { | ||
String xpath = "//div[@data-refer='metalake-name-field']"; | ||
WebElement nameField = driver.findElement(By.xpath(xpath)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we define a link element variable in class member?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
… content is loading for each small module (apache#2574) ### What changes were proposed in this pull request? 1. Displaying a fallback while content is loading for the main content Displaying a fallback while content is loading for metalakes list table body module Displaying a fallback while content is loading for catalog tree list modul ![UILoading](https://github.com/datastrato/gravitino/assets/9210625/0911c818-ee1b-400d-9b58-3af764a59363) 2. Define metadata icon src path based on the operating environment 3. Fix some issue of inconsistent behaviour - Smooth display of loaded data - Preserve search parameters when strongly refreshing a page ![fixbugs](https://github.com/datastrato/gravitino/assets/9210625/dd94fce7-4ee0-4c76-a40e-94e4a87f5ecc) ### Why are the changes needed? An instant loading state is fallback UI that is shown immediately upon navigation. You can pre-render loading indicators such as skeletons and spinners. This helps users understand the app is responding and provides a better user experience. Fix issue because of the need for consistent behaviour after refactoring Fix: apache#2570, apache#2559 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Will run e2e test after PR apache#2534 merged --------- Co-authored-by: CHEYNE <[email protected]>
…running on the github actions (apache#2534) ### What changes were proposed in this pull request? Fix web e2e test running. Testing on GitHub actions will result in errors, but it doesn't happen locally. It's probably due to performance issues in the testing environment causing the frontend delay animation to take longer than expected. ### Why are the changes needed? Fix: apache#2532 Fix: apache#2512 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Github Actions
What changes were proposed in this pull request?
Fix web e2e test running.
Testing on GitHub actions will result in errors, but it doesn't happen locally. It's probably due to performance issues in the testing environment causing the frontend delay animation to take longer than expected.
Why are the changes needed?
Fix: #2532
Fix: #2512
Does this PR introduce any user-facing change?
N/A
How was this patch tested?
Github Actions